fix(serve): parse tool calls with empty/omitted argument markers in ToolParser - #57
fix(serve): parse tool calls with empty/omitted argument markers in ToolParser#57Ultron09 wants to merge 1 commit into
Conversation
…oolParser When a tool call contains no arguments (e.g. <|tool_call_begin|>functions.func:0<|tool_call_end|>) or when a stream ends in header state, _parse_header() was previously bypassed because ARG_BEGIN was not encountered, causing the tool call to be silently dropped. Also catch EngineError in version() and build_info() when libwaste is unbuilt so server health endpoints and test runners degrade gracefully. Add regression tests in tests/serve/test_chatfmt.py.
mfethe1
left a comment
mfethe1
left a comment
There was a problem hiding this comment.
Validated on macOS 15.6 / arm64 against head cce7c67.
Full suite on PR head: python3 -m pytest tests/serve/test_chatfmt.py -q → 51 passed (main's 49 + the 2 new).
Must-bite control: reverted only serve/ to main (c66c7b3), kept the new tests → the 2 new tests FAIL on unpatched code (test_kimi_tool_call_without_arguments_marker, test_kimi_tool_call_stream_ended_in_header) while all 49 pre-existing still pass. The tests genuinely pin the fix, not the fixture.
Edge probes on PR head (beyond the author's tests):
- Two no-arg calls back-to-back →
[('get_time',0,''), ('ping',1,'')]— no duplication, indexes correct. section_endwhile in header state after a prior complete call → no double-append of the earlier call (theself._current is not Noneguard ordering is right).- Non-int index suffix (
functions.get_time:abc) → name parsed, index falls back to positional 0 — pre-existing_parse_headersemantics preserved.
Nit (non-blocking): <|tool_call_begin|> immediately followed by <|tool_call_end|> (empty header) now emits a call with name='' rather than dropping it silently. Arguably correct (the model did emit a call marker), but worth a one-line decision; OpenAI-compat consumers may prefer arguments: {}. Not a merge blocker for me.
engine.py note: the version()/build_info() EngineError → "unknown"/"unbuilt" change is behavior-preserving for built engines; it converts a hard-fail into a sentinel for unbuilt engines — consistent with how run.sh distinguishes engine-missing from engine-wrong. No regression observed.
Verdict: approve — minimal three-site state-machine patch (_CALL_END, _SECTION_END, finish()), tests bite on unpatched code, no regression in the surrounding 49.
2 participants
Summary
When Kimi / DeepSeek models generate parameterless tool calls or tool calls without explicit arguments (e.g.
<|tool_call_begin|>functions.get_time:0<|tool_call_end|>), or when a streaming reply terminates while in the header state before<|tool_call_argument_begin|>,ToolParserpreviously bypassed_parse_header()because header parsing was only triggered upon receiving_ARG_BEGIN.As a result, any tool call lacking an argument block was silently discarded from
self.callswhen_CALL_ENDor_SECTION_ENDarrived.Changes
serve/kimitools.py:ToolParser.feed_marker(): Check ifself._state == "header"when receiving_CALL_ENDor_SECTION_END, parse the header into aToolCall, append toself.calls, and finalize arguments (json_block).ToolParser.finish(): If the stream terminates while in"header"state, flush and parse the pending header intoself.calls.serve/engine.py:version()andbuild_info(): CatchEngineErrorwhenlibwasteshared library is unbuilt so server health endpoints and test runners degrade gracefully.tests/serve/test_chatfmt.py:test_kimi_tool_call_without_arguments_markertest_kimi_tool_call_stream_ended_in_headerVerification
python -m unittest tests.serve.test_chatfmt(51/51 tests pass).python -m unittest discover -s tests/serve -t . -p "test_*.py"(244/244 OK).